Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/7.0] [wasm] Fix JSExport on struct and records #74890

Merged
merged 11 commits into from
Sep 2, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 31, 2022

Backport of #74763 to release/7.0

/cc @maraf

Customer Impact

Fix using JSExport on structs, records and methods with expression body.

Testing

Unit test

Risk

None

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

@maraf maraf added the arch-wasm WebAssembly architecture label Aug 31, 2022
@maraf maraf added this to the 7.0.0 milestone Aug 31, 2022
@ghost
Copy link

ghost commented Aug 31, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #74763 to release/7.0

/cc @maraf

Customer Impact

Fix using JSExport on structs, records and methods with expression body.

Testing

Unit test

Risk

None

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript

Milestone: -

@carlossanlop
Copy link
Member

@maraf @lewing @pavelsavara this PR was created at noon PDT yesterday. I am unsure if it was able to finish before the CI went down for maintenance, which was around 4pm PDT. Can you please verify that the CI results ran the necessary tests and they passed?

@carlossanlop
Copy link
Member

Update: I was told the safe thing to do is to close and reopen PRs that may have been impacted by the system being down. The CI runs might have kept running during maintenance, but the results probably didn't flow to GitHub.

@carlossanlop carlossanlop reopened this Sep 1, 2022
@carlossanlop
Copy link
Member

All green now, with a proper amount of CI legs executed.
Approved and signed off.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit ed775e2 into release/7.0 Sep 2, 2022
@carlossanlop carlossanlop deleted the backport/pr-74763-to-release/7.0 branch September 2, 2022 00:05
@pavelsavara
Copy link
Member

Would this merge show up in RC2 ? @maraf I think we need to merge it there as separate PR

@maraf
Copy link
Member

maraf commented Sep 20, 2022

Yes, it shows up in RC2.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants